Skip to content

fix(security): reject non-local origins, bind loopback by default - #507

Open
nichochar wants to merge 2 commits into
mainfrom
fix/network-security
Open

fix(security): reject non-local origins, bind loopback by default#507
nichochar wants to merge 2 commits into
mainfrom
fix/network-security

Conversation

@nichochar

@nichochar nichochar commented Jul 29, 2026

Copy link
Copy Markdown
Contributor
packages/api/
  server/
    security.mts          +134/-0    origin allowlist, cors opts, verifyClient, bindHost
    http.mts              +28/-46    global cors + verifyOrigin, drop 41 per-route calls
    ws-client.mts         +11/-0     close disallowed origins as a backstop
  test/
    security.test.mts     +97/-0     12 tests
  dev-server.mts          +5/-3      bind host, verifyClient
  index.mts               +2/-1      export bindHost, webSocketServerOptions
srcbook/src/
  server.mts              +18/-6     bind host + warn, guard process.send
docker-compose.yml        +5/-1      HOST=0.0.0.0 inside container, explain why
turbo.json                +2/-0      declare HOST, SRCBOOK_ALLOWED_ORIGINS
tasks/0001-...md          +22/-1     close, record findings
.changeset/               +19/-0

Stacked on #506. Closes task 0001.

Srcbook has no authentication and executes arbitrary code, so the request origin is the only thing separating the user's own tab from any other page they have open. Nothing was checking it.

What was possible before this

Verified against a running instance, not theorized:

GET /api/settings from any origin returned every provider API key with Access-Control-Allow-Origin: *
POST /api/file from any origin read arbitrary paths off disk
WebSocket from any origin accepted, giving cell:update + cell:exec
Bind address *:2150 — the whole network, not just the machine

The websocket is what makes this remote code execution rather than disclosure: a page lists sessions over HTTP, writes a cell, runs it. No interaction beyond visiting the page.

What changed

New server/security.mts owns the allowlist — local origins on any port, plus anything in SRCBOOK_ALLOWED_ORIGINS. Both the HTTP and websocket paths use it, so they can't drift.

HTTP. The 41 per-route cors() calls collapse into one cors + verifyOrigin pair on the router. Disallowed origins get a 403, not just a missing CORS header — CORS alone stops an attacker reading a response, but the request still executes, which is all you need for POST /api/settings.

WebSocket. Origin is checked in verifyClient, before the handshake. I first put the check in the connection handler and confirmed it's too late: the server logged the rejection, but the client still saw open fire and only afterwards got closed. verifyClient gives a 401 at the handshake and no socket at all.

Bind. 127.0.0.1 unless HOST says otherwise, with a warning when it doesn't. This makes docker-compose's HOST variable real — it was previously read by nothing, while reading as though it controlled exposure. Compose now sets HOST=0.0.0.0, because Docker forwards published ports to the container IP; host-side exposure stays controlled by HOST_BIND, which already defaulted to loopback.

Bonus. Guarded process.send in the CLI server — it only exists under IPC, so node dist/src/server.mjs crashed immediately after printing running at http://localhost:2150. Found while testing the bind change; it's one line and in a file this PR already touches.

Verification

Re-ran the original attacks against the patched build:

foreign origin  → HTTP 403 / websocket 401 at handshake
localhost:2150  → HTTP 200 / websocket connects        (the app)
localhost:5173  → HTTP 200 / websocket connects        (vite dev)
no Origin       → HTTP 200 / websocket connects        (CLI, curl)
bind            → TCP 127.0.0.1:2150 (was TCP *:2150)

12 new tests for the allowlist, including the substring cases that make this kind of check go wrong (localhost.evil.com, example.com.evil.com). Suite is 23 tests, up from 11. Build, lint, and format all pass.

Compatibility

Anyone deliberately reaching Srcbook from another machine now needs HOST plus their origin in SRCBOOK_ALLOWED_ORIGINS. That's a real behavior change, hence minor in the changeset, and the whole point — it was previously open to the network by default.

Not covered here — task 0002, next in the stack: path traversal in DELETE /api/srcbooks/:id, shell injection via cell filenames, containing POST /api/file to SRCBOOKS_DIR, and validating the settings body.

🤖 Generated with Claude Code

@nichochar
nichochar force-pushed the docs/project-memory branch from b0af817 to ea93a9b Compare July 29, 2026 15:24
@nichochar
nichochar force-pushed the fix/network-security branch from 01d6839 to 9c1c3f0 Compare July 29, 2026 15:24
Base automatically changed from docs/project-memory to main July 29, 2026 16:42
Srcbook has no authentication and can execute arbitrary code, so the only
thing separating the user's own tab from any other page they have open is
the request origin. Nothing was checking it.

Verified before this change, against a running instance: any web page could
read every provider API key from GET /api/settings, read arbitrary files
from disk via POST /api/file, and — because the websocket accepted any
origin — open a socket and drive cell:update + cell:exec to run code as the
user. The server also bound 0.0.0.0, so all of it was reachable from the
whole network.

Changes:

- New server/security.mts owns the origin allowlist. Local origins on any
  port are allowed, plus anything in SRCBOOK_ALLOWED_ORIGINS. Requests with
  no Origin header are allowed, since browsers always set it on cross-origin
  requests and non-browser clients (the CLI, curl) legitimately omit it.
- HTTP: the 41 per-route cors() calls become one cors + verifyOrigin pair on
  the router. Disallowed origins get a 403 rather than just a missing CORS
  header — CORS alone stops an attacker reading a response, but the request
  still executes, which is all you need for POST /api/settings.
- WebSocket: origin checked via verifyClient, so rejection happens at the
  handshake. Checking in the connection handler is too late; the client
  observes an open socket that is only torn down afterwards.
- Bind 127.0.0.1 unless HOST says otherwise, and warn when it doesn't. This
  makes docker-compose's HOST variable real — it was previously read by
  nothing. Compose now sets HOST=0.0.0.0 because Docker forwards published
  ports to the container IP; host-side exposure stays controlled by
  HOST_BIND, which already defaulted to loopback.
- Guard process.send in the CLI server. It only exists under IPC, so running
  the server directly crashed it right after printing "running at ..." —
  which is how this was found while testing the bind change.

12 tests for the allowlist, covering the substring cases that make this kind
of check go wrong (localhost.evil.com, example.com.evil.com).

nichochar commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant